Prepared package to be used as a git submodule#17
Conversation
WalkthroughThis pull request removes two internal support libraries (PrincipleMacrosTestSupport and PrincipleMacrosClientSupport) from the Swift package manifest, deletes their source files, updates GitHub Actions workflows to use checkout v5 with submodules support and adds a mise installation step, refactors configuration files to use literal strings instead of template variables, and excludes additional paths from linting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pull-request.yml(5 hunks).github/workflows/release.yml(1 hunks).mise.toml(2 hunks).swiftlint.yml(1 hunks)Package.swift(0 hunks)Sources/PrincipleMacrosClientSupport/AccessControlLevel.swift(0 hunks)Sources/PrincipleMacrosTestSupport/Imports.swift(0 hunks)
💤 Files with no reviewable changes (3)
- Package.swift
- Sources/PrincipleMacrosClientSupport/AccessControlLevel.swift
- Sources/PrincipleMacrosTestSupport/Imports.swift
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-and-test (watchos)
- GitHub Check: build-and-test (macos)
- GitHub Check: build-and-test (visionos)
- GitHub Check: build-and-test (tvos)
- GitHub Check: build-and-test (ios)
🔇 Additional comments (5)
.swiftlint.yml (1)
166-167: LGTM!Adding the
Macros/Dependenciesexclusion aligns with the submodule setup and correctly prevents SwiftLint from analyzing those dependency files..github/workflows/release.yml (1)
15-18: LGTM!The checkout@v5 upgrade with
submodules: truecorrectly enables submodule initialization. This aligns with the PR objective and maintains the existingfetch-depth: 0for API compatibility checks..github/workflows/pull-request.yml (3)
24-26: LGTM!Both checkout steps correctly use actions/checkout@v5 with
submodules: true, ensuring submodules are initialized in both the prepare and build-and-test jobs.Also applies to: 75-77
31-36: LGTM!The new mise setup step correctly installs build tools needed for linting. This is appropriately placed in the prepare job before running linters.
119-120: The flag is appropriate for this project.The
-IDEPackageEnablePrebuilts=NOflag forces building Swift packages from source instead of using prebuilt artifacts, and is commonly used when prebuilt issues arise with macros and plugins. PrincipleMacros is precisely this type of project—a macro library extending SwiftSyntax. The recent commit "Prepared package to be used as a git submodule" added this flag alongside infrastructure updates (submodule-aware checkout), suggesting this is a known mitigation for swift-syntax prebuilt incompatibilities in Xcode builds across multiple platforms (iOS, tvOS, watchOS, visionOS, macCatalyst). The build time trade-off is intentional and standard for this use case. No compatibility issues are evident in the codebase.
| --config .swiftlint.yml \ | ||
| --strict \ | ||
| {{ vars.sources }} | ||
| Sources Macros |
There was a problem hiding this comment.
🧩 Analysis chain
Verify hardcoded paths match current project structure.
The migration from template variables to literal strings "Sources Macros" and "Tests" is clearer but now hardcoded. Ensure these paths correctly reflect the directory structure after removing PrincipleMacrosTestSupport and PrincipleMacrosClientSupport targets.
Run the following to verify these directories exist and are appropriate:
Also applies to: 31-31
🏁 Script executed:
#!/bin/bash
# Verify the paths used in .mise.toml exist
echo "Checking swiftlint:sources paths..."
ls -ld Sources 2>/dev/null && echo "✓ Sources exists" || echo "✗ Sources missing"
ls -ld Macros 2>/dev/null && echo "✓ Macros exists" || echo "✗ Macros missing"
echo -e "\nChecking swiftlint:tests path..."
ls -ld Tests 2>/dev/null && echo "✓ Tests exists" || echo "✗ Tests missing"
echo -e "\nVerifying Package.swift no longer references removed targets..."
grep -E "PrincipleMacrosTestSupport|PrincipleMacrosClientSupport" Package.swift && echo "✗ Old targets still referenced" || echo "✓ Old targets removed"Length of output: 313
🏁 Script executed:
#!/bin/bash
# Search for Macros-related targets and understand the project structure
echo "=== Searching Package.swift for Macros targets ==="
grep -n "\.target.*[Mm]acro" Package.swift | head -20
echo -e "\n=== Checking all top-level directories ==="
ls -ld */ 2>/dev/null | awk '{print $NF}'
echo -e "\n=== Searching for any Macros directory recursively ==="
find . -maxdepth 2 -type d -name "*[Mm]acro*" 2>/dev/null
echo -e "\n=== Viewing current .mise.toml around lines 20 and 31 ==="
sed -n '15,35p' .mise.tomlLength of output: 656
Fix hardcoded path: "Macros" directory does not exist at project root.
The migration to hardcoded literal "Sources Macros" introduced a configuration error. Project structure shows only Sources/ and Tests/ at the root level; macro-related code resides at Sources/PrincipleMacros/, not as a standalone Macros directory. Line 20 should reference only Sources (which encompasses PrincipleMacros as a subdirectory):
Sources
Alternatively, if macro linting requires a distinct path, adjust to Sources/PrincipleMacros, but verify swiftlint supports nested directory targeting.
🤖 Prompt for AI Agents
.mise.toml around line 20: the entry "Sources Macros" is a hardcoded/incorrect
path because there is no top-level Macros directory; update the line to
reference the correct directory—either "Sources" to include PrincipleMacros as a
subdirectory, or "Sources/PrincipleMacros" if linting needs an explicit macro
path—then verify the tool accepts nested paths (and use that form if required).
1 participant
Summary by CodeRabbit